-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Obs matrix #374
Obs matrix #374
Conversation
This PR is ready for review and merge. There are features such as caching of intermediate products that we will want to add but they are not critical right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impressive new tool! Going to be some work to port to toast3, but good to merge this sooner rather than later so that the work can begin.
|
||
void accumulate_observation_matrix(py::array_t <double, | ||
py::array::c_style | py::array::forcecast> c_obs_matrix, | ||
py::array_t <int64_t, py::array::c_style | py::array::forcecast> c_pixels, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at this code in the python bindings, we should collectively come up with some developer documentation about:
- When should we pass buffers as arguments vs. explicit numpy arrays? Are there performance or other consequences for one choice vs the other?
- How do we draw the line between calling
libtoast
functions from the bindings vs just putting that C++ code directly in the bindings? Should we just build the full library here? Should we move the bindings directly into the library?
I don't know the answer to these questions yet, but it does feel like we are using multiple "patterns" to accomplish the same thing throughout the bindings.
from ..todmap import OpFilterBin | ||
|
||
|
||
def add_filterbin_args(parser): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These options will all become Operator class traits in toast3
from ..timing import function_timer | ||
|
||
|
||
class OpFilterBin(Operator): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some notes here for when we port to toast3... The current BinMap
operator (
toast/src/toast/ops/mapmaker_binning.py
Line 29 in cfc6eab
class BinMap(Operator): |
Pipeline
of arbitrary other operators.
|
||
@function_timer | ||
def _get_detweights(self, data): | ||
# FIXME : We need a generic way of building the detector weights |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in toast3, every Noise
instance has a method to return the detector weights.
|
||
phase = self._get_phase(tod) | ||
t1 = time() | ||
common_templates = self._build_common_templates( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More notes for toast3 porting... It seems like the FilterBin
operator could be constructed like:
- operator which builds common templates for every observation and view
- operator which builds and regresses per-detector templates
- operator which accumulates observation matrix
- Pipeline which runs pointing expansion and (2.) - (3.) one detector at a time
Then the FilterBin
operator can internally run (1.) and then BinMap
passing the pipeline in (4.) as the pre_process
option. Anyway, I'm sure we'll iterate on this along the way.
Add a new operator to filter and bin signal:
OpFilterBin
. It is also able to calculate the matching observation matrix.